fix/add date-related functions#643
Conversation
krlmlr
left a comment
There was a problem hiding this comment.
Thanks! The bug fix and the new features could be two separate PRs. Also, medium-term, I'd like to switch to snapshot tests for the SQL translations, this PR could be a start.
| # clock | ||
| add_days = function(x, n, ...) { | ||
| build_sql("DATE_ADD(", !!x, ", INTERVAL '", n ," day')") | ||
| build_sql("DATE_ADD(", !!x, ", INTERVAL (", n ,") day)") |
There was a problem hiding this comment.
I now wonder why this says !!x with the bang-bang and n without.
There was a problem hiding this comment.
I guess is a choice, I think x can be a function to be evaluated in this case, while n can't
|
|
||
| }, | ||
| date_build = function(year, month = 1L, day = 1L, ..., invalid = NULL) { | ||
| dbplyr:::check_unsupported_arg(invalid, allow_null = TRUE) |
There was a problem hiding this comment.
I'd rather not access a function private to dbplyr here. How do other packages handle this? As a last resort, we could vendor a variant of check_unsupported_arg() here.
There was a problem hiding this comment.
Where would you implement such a function?
There was a problem hiding this comment.
Could be a new dbplyr.R or check_unsupported_arg.R .
| date_build = function(year, month = 1L, day = 1L, ..., invalid = NULL) { | ||
| dbplyr:::check_unsupported_arg(invalid, allow_null = TRUE) | ||
| rlang::check_dots_empty() | ||
| build_sql("MAKE_DATE(CAST(", year, " AS INTEGER), CAST(", month, " AS INTEGER), CAST(", day, " AS INTEGER))") |
There was a problem hiding this comment.
I see how SELECT MAKE_DATE(2022.0, 3.0, 8.0); fails in duckdb. Still, casting shouldn't be a concern to MAKE_DATE() .
| difftime = function(time1, time2, tz, units = "days") { | ||
| dbplyr:::check_unsupported_arg(tz) | ||
| dbplyr:::check_unsupported_arg(units, allowed = "days") | ||
| build_sql("DATEDIFF('day', ", !!time2, ", " ,!!time1, ")") |
There was a problem hiding this comment.
I wonder if we could use the recommended sql_expr() in the translation here and above.
3e5d5fa to
f344576
Compare
|
@IoannaNika: Do you want to take another stab? |
|
@krlmlr I can try to make some time to work on this. Should I work on a separate PR? If I recall correctly the tasks remaining are to add support for these functions:
|
|
Thanks. The comments above are still valid, adding to this PR is fine. |
|
@krlmlr I have issues building the package, I get some error:
Do you know why that is? |
|
When the code changes, I often need |
923f7d9 to
69dbe63
Compare
|
I think It's working now @krlmlr library(duckdb)
#> Loading required package: DBI
library(clock)
library(DBI)
library(dplyr)
#>
#> Attaching package: 'dplyr'
#> The following objects are masked from 'package:stats':
#>
#> filter, lag
#> The following objects are masked from 'package:base':
#>
#> intersect, setdiff, setequal, union
con <- DBI::dbConnect(duckdb::duckdb())
date_df <- dplyr::tibble(id = 1)
DBI::dbWriteTable(con, "tmpdate", date_df)
date_tbl <- dplyr::tbl(con, "tmpdate")
df <- dplyr::mutate(date_tbl,
date7 = clock::date_build(as.integer(1999)),
date8 = clock::date_build(as.integer(2000), as.integer(1), as.integer(1))
)
df
#> # Source: SQL [?? x 3]
#> # Database: DuckDB 1.4.2-dev16 [root@Darwin 24.0.0:R 4.4.1/:memory:]
#> id date7 date8
#> <dbl> <date> <date>
#> 1 1 1999-01-01 2000-01-01
df <- dplyr::mutate(df,
diff_time = difftime(date8, date7)
)
df
#> # Source: SQL [?? x 4]
#> # Database: DuckDB 1.4.2-dev16 [root@Darwin 24.0.0:R 4.4.1/:memory:]
#> id date7 date8 diff_time
#> <dbl> <date> <date> <dbl>
#> 1 1 1999-01-01 2000-01-01 365Created on 2025-10-15 with reprex v2.1.1 |
|
@krlmlr @IoannaNika could I check the status of this pr? I'd love to make use clock::date_build() translation :) |
Added support for:
Fixed:
The issue is shown below. When the functions are used with a variable being passed for the interval number to be added, the translation is wrong. Parenthesis are needed to support variables (see documentation https://duckdb.org/docs/sql/functions/date.html#date_adddate-interval)
Created on 2024-12-12 with reprex v2.1.1